-
Notifications
You must be signed in to change notification settings - Fork 24
fix: show warning when path contains delimiter (#6606) #259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
lib/set-path.js
Outdated
| if (binPaths) { | ||
| for (const bin of binPaths) { | ||
| if (bin.includes(delimiter)) { | ||
| log.warn('exec', 'Project path contains delimiter (":"), npx may not behave as expected.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not npx, this is used in other places. We will need to make the message more generic to cover all use cases (npm exec, npm run).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the input ! , Now I've modified the message to include the event that triggered this warning to provide useful information. Requesting your feedback on this.
| // guaranteed, nor is it guaranteed to be the only one. Merge them | ||
| // all together in the order they appear in the object. | ||
| const setPATH = (projectPath, binPaths, env) => { | ||
| const setPATH = (projectPath, binPaths, env, event) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are already carrying event in env.npm_lifecycle_event. We should pull it from there, instead of passing it in a second context.
Problem:
This pull request is aimed at npm/cli#6606. Currently if the project path includes a colon npx and npm exec will fail to find installed packages with no explanation.
Solution:
Added a warning message so that people can understand about the delimiter present in the path.
References